fix(jsonrpc): make the JSON-RPC output more compliant with specification#6763
Conversation
|
|
||
| JsonNode responseNode; | ||
| try { | ||
| responseNode = MAPPER.readTree(responseBytes); |
There was a problem hiding this comment.
[MUST] MAPPER is constrained by node.http.maxTokenCount/maxNestingDepth, but it is also used to parse server-generated batch sub-responses. A valid response below node.jsonrpc.maxResponseSize can exceed maxTokenCount and be converted into -32603 Internal error.
Please use the constrained mapper only for inbound request parsing, and use an unconstrained/internal mapper for response assembly and readTree(responseBytes). Add a regression test where a batch sub-response is below maxResponseSize but above maxTokenCount.
| rootNode = MAPPER.readTree(body); | ||
| if (rootNode == null || rootNode.isMissingNode()) { | ||
| writeJsonRpcError(resp, JsonRpcError.PARSE_ERROR, "Parse error", null, false); | ||
| writeJsonRpcError(resp, JsonRpcError.PARSE_ERROR, "JSON parse error", null, false); |
There was a problem hiding this comment.
[MUST] This changes the observable JSON-RPC error response, not just code style. JSON-RPC 2.0 defines the standard message for code -32700 as Parse error, and the PR is explicitly moving responses toward spec compliance. Please keep the previous Parse error message here.
What does this PR do?
Fixes three JSON-RPC spec-compliance issues and one parser-hardening gap in
JsonRpcServlet:1.
Content-Typeresponse header — remove spurious; charset=utf-8The JSON-RPC 1.0/2.0 spec and Ethereum JSON-RPC both use
application/jsonorapplication/json-rpcwithout a charset suffix. The extra; charset=utf-8suffix was non-standard and has been removed.2. Non-object/non-array root node →
Invalid RequestSending a JSON primitive as the request body (e.g.
null,true,123) was silently forwarded tojsonrpc4j, which produced a malformed response with"id":"null"(string) instead of"id":null(JSON null). Now these cases are caught before dispatch and return a proper-32600 Invalid Requestresponse with"id":null.Before:
After:
3. Non-object element inside a batch →
Invalid RequestA batch containing a non-object element (e.g.
[[]]) was forwarded per-element tojsonrpc4j, which echoed the inner array back unchanged, producing[[]]instead of an error. Non-object sub-requests are now rejected with-32600 Invalid Requestbefore dispatch.Before:
After (Ethereum-compatible):
4. Apply
StreamReadConstraintsto the JSON-RPC parserJsonRpcServletused a barenew ObjectMapper(), so thenode.http.maxNestingDepthandnode.http.maxTokenCountsettings introduced in #6701 had no effect on JSON-RPC requests. The mapper is now built with aJsonFactorythat reads those two limits fromCommonParameter, consistent with howJSON.javaconstructs its mapper.Without this fix, a ~4 MB request body such as
[0,0,...,0](~2 M tokens) could bypass the configuredmaxTokenCount(default 100,000) and force Jackson to allocate ~2 MJsonNodeobjects, causing GC pressure (Eden/Survivor saturation → premature promotion → Full GC). With the fix, the parser throws after the configured token limit is reached, well before the 4 MB body-size cap.Why are these changes required?
node.http.maxNestingDepth/node.http.maxTokenCountconfig knobs actually effective for the JSON-RPC endpoint, closing a token-density amplification vector.This PR has been tested by
Extra details
Issue 4 shares the same root cause identified in the review comment on this PR: the
MAPPERinJsonRpcServletwas not wired to theStreamReadConstraintsconfigured vianode.http.*.